Add reasoning effort controls to web UI#84
Conversation
Greptile OverviewGreptile SummaryThis PR wires a new Main things to fix before merging:
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -545,6 +546,7 @@ export function useSessionSocket(sessionId: string): UseSessionSocketReturn { | |||
| type: "prompt", | |||
| content, | |||
| model, // Include model for per-message model switching | |||
| reasoningEffort, | |||
| }) | |||
| ); | |||
| }, []); | |||
There was a problem hiding this comment.
Stale closure in sendPrompt
sendPrompt is declared with an empty dependency array but reads wsRef.current, subscribedRef.current, and setSessionState. This was already risky before, but the new retry path now also closes over reasoningEffort (and model). If sendPrompt is called before subscription completes, the setTimeout will re-invoke sendPrompt with whatever reasoningEffort value was captured at the time of the first call, even if the user changes it before the retry fires. Fix by adding the needed dependencies (or storing the latest args in refs and reading from those in the timeout) so retries send the current model/effort and don’t rely on stale closure state.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/web/src/hooks/use-session-socket.ts
Line: 521:552
Comment:
**Stale closure in `sendPrompt`**
`sendPrompt` is declared with an empty dependency array but reads `wsRef.current`, `subscribedRef.current`, and `setSessionState`. This was already risky before, but the new retry path now also closes over `reasoningEffort` (and `model`). If `sendPrompt` is called before subscription completes, the `setTimeout` will re-invoke `sendPrompt` with whatever `reasoningEffort` value was captured at the time of the first call, even if the user changes it before the retry fires. Fix by adding the needed dependencies (or storing the latest args in refs and reading from those in the timeout) so retries send the current model/effort and don’t rely on stale closure state.
How can I resolve this? If you propose a fix, please make it concise.
Import model definitions from @open-inspect/shared, replacing local MODEL_OPTIONS. Add reasoning effort pill buttons (high/max) to both home and session pages. Thread reasoningEffort through WebSocket hook, session creation API, and prompt API. Display effort level in the session sidebar metadata.
ColeMurray
force-pushed
the
feat/reasoning-effort-phase2
branch
from
5c1cdde to
2cb2ef5
Compare
- Extract duplicated reasoning effort pill UI into shared ReasoningEffortPills component - Replace inline setSelectedModel wrappers with named handleModelChange callbacks - Remove redundant ?? undefined coercions (getDefaultReasoningEffort already returns undefined) - Replace local ModelOption interface with shared ModelDisplayInfo type
Clicking the text cycles through available efforts (e.g. high → max), keeping the footer compact as more reasoning modes are added.
There was a problem hiding this comment.
Pull request overview
This PR adds “reasoning effort” as a first-class setting in the web UI (home + session pages) and threads it through session creation and prompt submission so the control plane can apply the appropriate reasoning configuration per request.
Changes:
- Replace duplicated in-web model lists by importing
MODEL_OPTIONS(and reasoning helpers) from@open-inspect/shared. - Add reasoning effort pill controls (e.g.,
high/max) to home and session footers, defaulting on model change. - Plumb
reasoningEffortthrough the WebSocket prompt message, session creation API route, and prompt API route; display it in the session sidebar metadata.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web/src/hooks/use-session-socket.ts | Adds reasoningEffort to session state typing and includes it in WS prompt messages. |
| packages/web/src/components/sidebar/metadata-section.tsx | Displays reasoning effort next to the formatted model name in sidebar metadata. |
| packages/web/src/components/session-right-sidebar.tsx | Threads reasoningEffort from session state into MetadataSection. |
| packages/web/src/components/reasoning-effort-pills.tsx | New pill-button UI component for selecting reasoning effort per model. |
| packages/web/src/app/session/[id]/page.tsx | Uses shared model options, adds reasoning pills in the footer, resets default effort on model changes, and passes effort to sendPrompt. |
| packages/web/src/app/page.tsx | Uses shared model options, adds reasoning pills, and includes effort in session creation + initial prompt submission. |
| packages/web/src/app/api/sessions/route.ts | Passes reasoningEffort through to the control plane on session creation. |
| packages/web/src/app/api/sessions/[id]/prompt/route.ts | Passes reasoningEffort through to the control plane on prompt submission. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove reasoningEffort from warming session deps (per-message attribute, no need to recreate session on toggle) - Replace unsafe `as never` cast with `as ReasoningEffort` in indexOf - Add comment explaining implicit fallback to index 0 when effort not found
Summary
@open-inspect/shared, removing duplicate localMODEL_OPTIONSfrom both home and session pageshigh/max) alongside the model selector in both home page and session page footersmax) when model changesreasoningEffortthrough WebSocket hook (sendPrompt), session creation API, and prompt API to the control planeDepends on #83 (Phase 1: shared models + control plane + bridge).
Test plan
high,max) appear withmaxpre-selectedhigh— verify the value flows through session creation and prompt submission